fix(async-hooks): share ALS context with runtime provider - #8042
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe runtime adds a C ABI for ChangesAsyncLocalStorage propagation
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: ⚪ Minimal · up to This change moves async-local context handling behind the runtime provider while preserving throw-safe scope restoration and adding focused continuation coverage. No actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant RouteHandlers
participant AsyncLocalStorage
participant RuntimeABI
participant AsyncContinuations
RouteHandlers->>AsyncLocalStorage: enter action, request, work, and work-unit stores
AsyncLocalStorage->>RuntimeABI: register stores and scope guards
RouteHandlers->>AsyncContinuations: run imports, promises, timers, microtasks, and streams
AsyncContinuations->>AsyncLocalStorage: read propagated stores
RouteHandlers->>AsyncLocalStorage: exit nested scope and handle rejection
AsyncLocalStorage->>RuntimeABI: restore or clear context
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
proggeramlug
left a comment
There was a problem hiding this comment.
Audited exact head d93adeaa31de1562b541e57acf35ff7991fb3349.
The runtime-owned ALS provider ABI is internally coherent, and the focused unit test async_context::tests::als_provider_abi_keeps_nested_run_and_exit_scopes_balanced passes locally. I am holding the merge because this head claims to close #8037 without satisfying that issue's acceptance contract.
#8037 requires the pinned #8034 production AppRouteRouteModule.handle path, 20 concurrent requests, 100 repetitions, both executable and app-only dylib modes, and no direct-handler fallback. This PR exercises only the lower-level provider ABI and a focused parity fixture; the PR body itself describes it as the lower-level half.
Please either add and run the required production integration gate, or narrow the PR to a non-closing partial fix (Refs #8037 rather than Closes #8037). I can re-audit the updated exact head.
Summary
AsyncLocalStoragecontext mutation/read operations behind an explicit C ABI owned by the runtime providerrun()/exit()throw-safe restoration in the runtime image so promise, timer, microtask, dynamic-import, and stream schedulers snapshot the sameACTIVE_CONTEXTThis is a non-closing, focused lower-level part of #8037. The pinned production
AppRouteRouteModule.handleintegration gate and 20-request verifier remain tracked by #8034; this PR does not add a direct-handler compatibility path or claim that production acceptance gate.Validation
cargo fmt --all -- --checkcargo test --profile perry-dev -p perry-runtime async_context::tests::als_provider_abi_keeps_nested_run_and_exit_scopes_balanced -- --exact --nocapturePERRY_SKIP_BUILD=1 ... ./run_parity_tests.sh --suite node-suite --module async_hooks --filter next-route-continuations--output-type dylib, loaded by a C host, and matched its Node oracleconcurrent-promises,promise-microtask,promise-catch-finally, andnexttick-timerNo version bump.
Refs #8037